Skip to content

Fix Zod v4 schema description crash in agent-runtime - #1201

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-agent-runtime-zod-schema
Open

Fix Zod v4 schema description crash in agent-runtime#1201
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-agent-runtime-zod-schema

Conversation

@nordicnode

Copy link
Copy Markdown

Problem & Context

In packages/agent-runtime, ensureJsonSchemaCompatible(schema: z.ZodType) recovers from non-serializable schemas (such as z.function()) by generating a passthrough fallback object and copying the schema's description:

return schema.description ? fallback.describe(schema.description) : fallback

Under Zod v4, accessing the .description getter on a function schema evaluates schema._zod.parent. Because parent is undefined for function schemas, this throws an unhandled TypeError: undefined is not an object (evaluating 'schema._zod.parent') inside the recovery handler, crashing getToolSet and failing unit tests in prompts-schema-handling.test.ts.

Additionally, under Zod v4, .and() directly merges intersected object schema properties into the top-level properties instead of wrapping them in an allOf construct.

Changes Made

  • Guarded schema.description access in a nested try / catch block within ensureJsonSchemaCompatible() in both packages/agent-runtime/src/tools/prompts.ts and packages/agent-runtime/src/templates/prompts.ts, ensuring non-serializable schemas return the fallback object cleanly without throwing.
  • Updated prompts-schema-handling.test.ts to assert that converted MCP parameter properties (name, cb_easp) are preserved.

Architecture & Conventions Conformance

  • Adheres to Dependency Injection (contracts defined in common/src/types/contracts/, no module monkey patching)
  • Terminal commands use terminalCommandBroker (no direct spawn or TUI-process bypass)
  • Environment hygiene respected (getCliEnv() for CLI, getSdkEnv() for SDK, no forbidden getProcessEnv() imports)
  • Freebuff mode compatibility (IS_FREEBUFF preserved, no paid features introduced)
  • Imports ordered and explicit (import type used for types)

Scope Verification

  • All modified files are within allowed public directories: packages/agent-runtime/
  • NO modifications to web/, freebuff/web/, packages/internal/, packages/billing/, packages/bigquery/, or packages/build-tools/

Testing & Verification

  • bun test packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts passed 15/15 tests (was failing 2 tests).
  • bun run build:sdk passed cleanly.
  • bun run build:freebuff passed cleanly.
  • bun cli/scripts/smoke-binary.ts cli/bin/freebuff passed cleanly.
  • Anti-flake principles observed.

@codebuff-team

Copy link
Copy Markdown
Contributor

Good diagnosis: schema.description becoming a throwing getter on Zod v4 function schemas inside an already-degraded fallback path is a real bug, and wrapping it in try/catch in both packages/agent-runtime/src/tools/prompts.ts and .../templates/prompts.ts is the right layer to fix it — this is exactly the kind of defensive recovery code that should never itself throw. Small, in-scope, and easy to port.

Two things worth tightening before this lands:

  1. The nested try { ... } catch { return fallback } inside the outer catch works but reads awkwardly. Consider computing the description defensively up front, e.g. let description: string | undefined; try { description = schema.description } catch {}, then return description ? fallback.describe(description) : fallback. Same fix, clearer intent, and it's identical in both files so a small shared helper might be worth it too (duplicated logic across tools/prompts.ts and templates/prompts.ts already existed before this PR, but it's growing).

  2. The test change removes the expect(description).toContain('allOf') assertion and renames the test rather than adding a new test for the actual crash being fixed (calling ensureJsonSchemaCompatible on a z.function() schema with a .describe() on it, which is the actual repro from your bug report). Right now the PR's own regression — the .description getter throwing — isn't directly exercised by any test; the diff only patches an unrelated MCP-params test to match a Zod v4 behavior change in .and(). Please add a test that would fail on the pre-patch code (a described function schema hitting ensureJsonSchemaCompatible) so this doesn't regress silently on the next Zod bump.

Substance is right; needs a direct regression test for the crash itself before it's fully port-ready.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants